feat(premium-analytics): port components package from next-woocommerce-analytics#49360
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
0885ed3 to
0472569
Compare
…cs components Verbatim copy of packages/components from next-woocommerce-analytics (source, SCSS, stories, manifest, tsconfig), with only the directory renamed components -> ui: @wordpress/build hard-codes an Emotion Babel pass for any package directory literally named "components", which breaks TS transpilation; these components use SCSS, not Emotion. No content changes - later commits adapt the package to the monorepo.
…r monorepo Mechanical port adaptations, no behavior changes: - Import specifiers: @next-woo-analytics/datetime -> @jetpack-premium-analytics/datetime, @wc-analytics/formatters -> @jetpack-premium-analytics/formatters - Text domain: woocommerce-analytics -> jetpack-premium-analytics - Manifest: internal-packages name/private/types conventions, pinned versions matching siblings, drop @next-woo-analytics/data and @automattic/design-system (declared upstream, imported nowhere), declare @wordpress/i18n and react (imported but undeclared upstream), move date-fns to devDependencies (story-only import), add @storybook/react for the stories - Drop upstream wpModule/exports build fields: main/types point at the TS source like the sibling internal packages; the script-module build is deferred until the leaves are Node-resolvable - Stories: @storybook/react-vite types -> @storybook/react, titles follow the monorepo convention (Packages/Premium Analytics/UI/<Component>) - Delete leaf tsconfig.json: the parent tsconfig already includes packages/**/* and supplies the @jetpack-premium-analytics/* path alias (mirrors packages/datetime and packages/formatters)
…nlock @automattic/admin-toolkit is not available in this monorepo. Replace its unlock with a local src/lock/unlock.ts that opts in to @wordpress/private-apis directly, mirroring existing Jetpack precedent (jetpack-mu-wpcom getUnlock(), js-packages/charts stories). Swap the manifest dependency accordingly. The typed local helper also makes upstream's @typescript-eslint/no-unsafe-assignment disables unused (the repo lints unused disable directives), so drop them at the three unlock call sites.
…ngePresets DateComparisonDropdown reuses DateRangePresets (typed for primary presets) to render comparison presets. Upstream's looser tsc allowed the mismatch; the repo's tsgo does not. Cast the two props at the one call site - the component only reads id/label/range to render, so the runtime shape matches.
The design-system version shipped with the repo's @wordpress/ui renamed the font tokens: --wpds-font-size-* -> --wpds-typography-font-size-* and --wpds-font-line-height-* -> --wpds-typography-line-height-*. Upstream's names resolve to nothing here, so the inputs and preset rows would lose their sizing.
…mponents Upstream stories call useState inside story render callbacks, which the repo's react-hooks/rules-of-hooks rejects (a render callback is not a component). Extract each stateful render body into a named *WithState component; WithoutPrefix reuses DateComparisonDropdownWithState via a new removeCompareToPrefix prop instead of duplicating its body.
…ting Pure formatting, no content changes: prettier (line wrapping, arrow parens, paren spacing, scss quote style) plus eslint/stylelint autofix output (import order, import/first, scss blank-line and paren-spacing rules). The upstream external/internal dependency banner comments are dropped in the stories where the repo's import order interleaves the two groups.
… port Parent-level wiring for the ui leaf (leaves are not pnpm workspace members, so the parent's dependencies are the load-bearing ones): - package.json: add the runtime deps the components import (@automattic/ui, @wordpress/components, @wordpress/compose, @wordpress/private-apis, clsx; move @wordpress/ui to dependencies) plus @wordpress/base-styles (dev) so the SCSS @use resolves at build - eslint.config.mjs: temporary packages/ui/** block softening JSDoc rules and react/jsx-no-bind so the upstream style lands as-is; tracked for follow-up alongside datetime/formatters - changelog entry
c49c726 to
ce8c516
Compare
| @media not ( prefers-reduced-motion: reduce ) { | ||
|
|
||
| .date-comparison-dropdown__popover { | ||
| view-transition-name: next-admin--date-comparison-dropdown; |
There was a problem hiding this comment.
I think we should remove/replace these ids
There was a problem hiding this comment.
Renamed the next-admin-- view-transition ids to jp-premium-analytics-- across both SCSS files
There was a problem hiding this comment.
good catch, and yup agreed!
…e-components-package-into-analytics # Conflicts: # projects/packages/premium-analytics/eslint.config.mjs
dognose24
left a comment
There was a problem hiding this comment.
Reviewed the full port (incl. the bf3248b7dc view-transition rename — nice catch, that fixes an upstream next-admin namespace leak the verbatim copy carried in). Port fidelity is clean: no stale @next-woo-analytics/@wc-analytics/admin-toolkit specifiers, all text domains correct, the DateRangePresets cast is well-justified, and the local unlock helper follows existing Jetpack precedent. Two non-blocking suggestions inline.
| "@wordpress/components": "33.1.0", | ||
| "@wordpress/compose": "7.46.0", | ||
| "@wordpress/i18n": "^6.9.0", | ||
| "@wordpress/icons": "^13.0.0", | ||
| "@wordpress/private-apis": "1.46.0", |
There was a problem hiding this comment.
[suggestion] These three pins (@wordpress/components 33.1.0, @wordpress/compose 7.46.0, @wordpress/private-apis 1.46.0) are older than what the parent manifest resolves (35.0.0 / 8.1.0 / 1.48.0), and the sibling data leaf matches the parent exactly — so the PR description's "versions match siblings" doesn't hold for these.
No runtime impact today since the leaf isn't a pnpm workspace member, but when it becomes load-bearing in the first-consumer PR, pnpm would install a second, older @wordpress/components alongside the parent's. The private-apis skew is the riskiest one: its lock/unlock is version-coupled to @wordpress/components, so a mismatched pair can throw at runtime.
Suggest bumping the three specifiers to match the parent (35.0.0 / 8.1.0 / 1.48.0).
| padding-bottom: var(--wpds-dimension-gap-sm); | ||
| width: calc(60 * var(--wpds-dimension-base)); | ||
| padding-right: var(--wpds-dimension-padding-sm); |
There was a problem hiding this comment.
[suggestion] (low, fine to defer) A few physical-direction properties survived the verbatim copy (padding-bottom/padding-right here, padding-right in date-range-input.scss:10) in files that otherwise use logical properties (padding-block-start). RTL-incorrect, but since the cleanup follow-up is already tracked alongside the eslint relaxations, a logical-properties pass there would cover it — not blocking this PR.
Proposed changes
Related: WOOA7S-1318
First UI leaf in M2 — Shared Packages Integration: port
@next-woo-analytics/componentsinto@automattic/jetpack-premium-analyticsas the internaluipackage. Provides the date-filtering UI consumed by analytics widgets — the date-range popover (calendar + presets + manual inputs), the comparison dropdown, and theDateFiltersPanelcontainer that ties them together — plus the SCSS and Storybook stories for all of it. This is the first ported package that ships React components and styles.Commits
Reviewed most easily commit-by-commit — commit 1 is a byte-identical copy of upstream (including the three stories), so every later diff shows exactly what the monorepo port changed.
604a8c1334components→ui4becf55eb3e14415029aunlockhelper and dep swap78eb626d7ftsgof4114732d4--wpds-font-*→--wpds-typography-*)2c0fc06968rules-of-hooksfix in the three storiescd23fb79aba414d088bcpackage.json/eslint.config.mjs/lockfile + changelogce8c516591Monorepo adaptations
packages/componentspackages/ui@wordpress/buildhard-codes an Emotion Babel pass for any package directory literally namedcomponents(needsEmotionPlugin = packageName === 'components'), which runs Babel without a TypeScript preset and breaks TS transpilation. These components use SCSS, not Emotion, so the pass is incidental — naming the diruilets esbuild transpile the TS natively with no extra Babel toolingname: @next-woo-analytics/componentsname: @automattic/jetpack-premium-analytics-ui@jetpack-premium-analytics/ui(from the tsconfig path alias), separate from thename:field… from '@next-woo-analytics/datetime'(11 imports)… from '@jetpack-premium-analytics/datetime'… from '@wc-analytics/formatters'… from '@jetpack-premium-analytics/formatters'formatDate+formatDateRangeare used)import { unlock } from '@automattic/admin-toolkit'(4 files incl. one story)src/lock/unlock.tsvia@wordpress/private-apisadmin-toolkitisn't available here. The helper opts in with__dangerousOptInToUnstableAPIsOnlyForCoreModulesto reach the privateMenuof@wordpress/components— mirrors existing Jetpack precedent (jetpack-mu-wpcomgetUnlock,js-packages/chartsstories)@next-woo-analytics/data,@automattic/design-systemindependencies@wordpress/i18n,reactundeclareddate-fnsindependenciesdevDependencies--wpds-font-size-*,--wpds-font-line-height-*tokens--wpds-typography-font-size-*,--wpds-typography-line-height-*@wordpress/uirenamed the font tokens; the upstream names resolve to nothing here'woocommerce-analytics'text domain'jetpack-premium-analytics'tsconfig.jsonincludes: [packages/**/*]and supplies the@jetpack-premium-analytics/*path alias; mirrorspackages/datetime/packages/formatters*version specs@automattic/ui 1.0.2,@wordpress/components 33.1.0,@wordpress/compose 7.46.0,@wordpress/ui 0.13.0,@wordpress/private-apis 1.46.0,clsx 2.1.1)style:commit)One small TS adaptation worth flagging (own commit,
78eb626d7f):DateComparisonDropdownreusesDateRangePresets(typed for primary presets) to render comparison presets. Upstream's loosertscallowed it; the repo'stsgodoes not, so the two props are cast at that one call site with a comment — the component only readsid/label/rangeto render, so the runtime shape matches.Parent-level wiring (
projects/packages/premium-analytics/):package.json: adds the real runtime deps imported by the components (@automattic/ui,@wordpress/components,@wordpress/compose,@wordpress/private-apis,@wordpress/ui,clsx;@wordpress/icons/date-fnsalready present), plus@wordpress/base-styles(dev) so the SCSS@use "@wordpress/base-styles/…"resolves at build time. The leaf isn't a pnpm workspace member (the root glob doesn't reach two levels in), so the parent's deps are the load-bearing ones. Nolink:dependency is added yet — nothing importsui(same as datetime/formatters); the first consumer will add it.eslint.config.mjs(temporary): adds apackages/ui/**-scoped block (the datetime/formatters blocks are untouched) softening the JSDoc rules and relaxingreact/jsx-no-bindso the upstream inline-handler JSX style lands as-is. Tracked for follow-up alongside datetime/formatters.Build / SCSS
pnpm build(wp-build) transpiles every internal package;uinow transpiles cleanly and its SCSS compiles —@use "@wordpress/base-styles/colors"resolves ($gray-300→#ddd),--wpds-*tokens and class names are emitted, and the compiled CSS is inlined into the per-component output as a runtime style injection. Like datetime/formatters, the package is a library (not a build target), so it only lands inbuild/once a route imports it; building it standalone confirms the JS and styles compile.Storybook
The three upstream stories live in
packages/components/src/*/stories/and are copied verbatim in the first commit, then adapted in the same commits as the rest of the port:4becf55eb3):@storybook/react-vitetypes →@storybook/react; titles follow the icons convention (Packages/Premium Analytics/UI/<Component>);@storybook/react+date-fnsadded to the leafdevDependencies(story-only imports, mirroring the icons package — no lockfile change, leaves aren't pnpm importers).e14415029a): the presets story'sunlockcomes from the localsrc/lock/unlock.tshelper, as in the components themselves.2c0fc06968): inlinerender:bodies that call hooks are extracted into named*WithStatecomponents — upstream's pattern fails the repo'sreact-hooks/rules-of-hooks.The monorepo Storybook already picks up
premium-analytics/packagesstories (same glob the icons package uses). One storybook-project change was needed (ce8c516591): Storybook's vite had no resolution for the@jetpack-premium-analytics/*internal-package specifier — the tsconfig path alias only covers types and wp-build only covers the plugin build (the icons stories worked only because they have no cross-package imports).projects/js-packages/storybook/storybook/main.jsnow aliases@jetpack-premium-analyticstoprojects/packages/premium-analytics/packages, mirroring wp-build's mapping — each leaf'smainpoints at its TS source. Changelog entry included.Verified in
storybook:dev: all stories under Packages/Premium Analytics/UI render with SCSS applied (popover calendar + presets + manual inputs, comparison dropdown, presets menu), and the autodocs pages load with no console errors.What's intentionally not here
wpModule: true/ script-module build wiring — upstream declareswpModule: true, but adding the@wordpress/buildequivalent here breakspnpm build: the@jetpack-premium-analytics/*specifier is a tsconfig-paths-only alias, not Node-resolvable (the leaves aren't pnpm workspace members). Same deferral as the datetime/formatters ports; revisit when the leaves become resolvable packages.@automattic/admin-toolkit— replaced by the localunlockhelper.@next-woo-analytics/data/@automattic/design-system— declared upstream, imported nowhere.link:dependency on the parent —uiisn't imported anywhere yet, so wiring it into the build is deferred to the first consumer (mirrors datetime/formatters).eslint.config.mjscomments.Does this pull request change what data or activity we track or use?
No.
Testing instructions
Requires Node 24 (repo
engineStrict).